Skip to content

Conversation

@sanderegg
Copy link
Member

@sanderegg sanderegg commented Aug 19, 2025

What do these changes do?

Collaborative services can be opened by multiple users (e.g. currently only jupyter-math 3.1.0)
the locked state of the NodeUpdate event will be set accordingly.

Related issue/s

How to test

Dev-ops

@sanderegg sanderegg added this to the Voyager milestone Aug 19, 2025
@sanderegg sanderegg self-assigned this Aug 19, 2025
@sanderegg sanderegg added the a:webserver webserver's codebase. Assigning the area is particularly useful for bugs label Aug 19, 2025
@sanderegg sanderegg force-pushed the i1975/unlock-collaborative-nodes branch from 039d59e to e7414dd Compare August 19, 2025 15:50
@codecov
Copy link

codecov bot commented Aug 19, 2025

Codecov Report

❌ Patch coverage is 88.88889% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 89.83%. Comparing base (c6999c2) to head (a18226e).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #8236      +/-   ##
==========================================
+ Coverage   87.98%   89.83%   +1.85%     
==========================================
  Files        1917     1508     -409     
  Lines       74166    62297   -11869     
  Branches     1301      498     -803     
==========================================
- Hits        65253    55966    -9287     
+ Misses       8521     6202    -2319     
+ Partials      392      129     -263     
Flag Coverage Δ
integrationtests 63.94% <42.85%> (+0.02%) ⬆️
unittests 88.21% <66.66%> (+1.59%) ⬆️
Components Coverage Δ
pkg_aws_library ∅ <ø> (∅)
pkg_celery_library ∅ <ø> (∅)
pkg_dask_task_models_library ∅ <ø> (∅)
pkg_models_library 93.05% <100.00%> (+<0.01%) ⬆️
pkg_notifications_library ∅ <ø> (∅)
pkg_postgres_database ∅ <ø> (∅)
pkg_service_integration 70.19% <ø> (ø)
pkg_service_library ∅ <ø> (∅)
pkg_settings_library ∅ <ø> (∅)
pkg_simcore_sdk 85.03% <ø> (+0.05%) ⬆️
agent 93.90% <ø> (ø)
api_server 92.84% <ø> (ø)
autoscaling 95.89% <ø> (ø)
catalog 92.34% <ø> (ø)
clusters_keeper 99.13% <ø> (ø)
dask_sidecar 92.15% <ø> (ø)
datcore_adapter 97.94% <ø> (ø)
director 75.90% <ø> (+0.08%) ⬆️
director_v2 91.01% <75.00%> (ø)
dynamic_scheduler 96.27% <ø> (ø)
dynamic_sidecar 90.05% <ø> (ø)
efs_guardian 89.62% <ø> (ø)
invitations 91.44% <ø> (ø)
payments 92.61% <ø> (ø)
resource_usage_tracker 92.13% <ø> (+0.42%) ⬆️
storage 86.80% <ø> (+0.24%) ⬆️
webclient ∅ <ø> (∅)
webserver 88.05% <100.00%> (+0.02%) ⬆️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c6999c2...a18226e. Read the comment docs.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@mergify
Copy link
Contributor

mergify bot commented Aug 19, 2025

🧪 CI Insights

Here's what we observed from your CI run for a18226e.

✅ Passed Jobs With Interesting Signals

Pipeline Job Signal Health on base branch Retries 🔍 CI Insights 📄 Logs
CI [sys] e2e (3.11, 14, ubuntu-24.04) Base branch is healthy, but retries were needed. Could be early signs of flakiness 👀 Healthy 1 View View
system-tests Base branch is healthy, but retries were needed. Could be early signs of flakiness 👀 Healthy 1 View View
unit-tests Base branch is healthy, but retries were needed. Could be early signs of flakiness 👀 Healthy 1 View View

@sanderegg sanderegg force-pushed the i1975/unlock-collaborative-nodes branch 4 times, most recently from 310885c to dd622da Compare August 21, 2025 15:52
@sanderegg sanderegg requested review from Copilot and odeimaiz August 21, 2025 16:37
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR implements collaborative access for dynamic services by allowing multiple users to simultaneously access services that support collaboration (currently only jupyter-math 3.1.0). The changes primarily focus on modifying the node locking logic to check if a service is collaborative before applying locks, and include various code formatting improvements.

Key changes:

  • Modified node sharing logic to allow access to collaborative services
  • Added is_collaborative field to track service collaboration capability
  • Applied code style improvements across multiple files

Reviewed Changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
services/web/server/src/simcore_service_webserver/projects/_projects_service.py Updated node share state logic to check collaborative status and formatting improvements
services/web/server/src/simcore_service_webserver/projects/_crud_api_read.py Function call formatting improvement
services/web/server/src/simcore_service_webserver/projects/_crud_api_create.py Function call formatting improvement
services/web/server/src/simcore_service_webserver/projects/_controller/projects_states_rest.py Function call formatting improvement
services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/scheduler/_core/_scheduler_utils.py Added is_collaborative field to model creation
services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/docker_service_specs/sidecar.py Assert statement formatting improvement
services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/docker_service_specs/proxy.py Assert statement formatting improvement
services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/docker_compose_specs.py Variable assignment and function call formatting improvements
services/autoscaling/tests/unit/test_modules_cluster_scaling_dynamic.py Multiple assert statement formatting improvements
services/autoscaling/tests/unit/conftest.py Multiple assert statement formatting improvements
packages/models-library/tests/test_docker.py Assert statement formatting improvement
packages/models-library/src/models_library/api_schemas_directorv2/dynamic_services_service.py Added is_collaborative field definition to RunningDynamicServiceDetails

@sanderegg sanderegg marked this pull request as ready for review August 21, 2025 16:41
Copy link
Collaborator

@matusdrobuliak66 matusdrobuliak66 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thanks.

Copy link
Member

@odeimaiz odeimaiz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🪞

@sanderegg sanderegg added the 🤖-automerge marks PR as ready to be merged for Mergify label Aug 22, 2025
@sanderegg
Copy link
Member Author

@mergify queue

@mergify
Copy link
Contributor

mergify bot commented Aug 22, 2025

queue

🟠 Waiting for conditions to match

  • any of: [🔀 queue conditions]
    • all of: [📌 queue conditions of queue default]
      • branch-protection-review-decision = APPROVED [🛡 GitHub branch protection]
      • any of: [🛡 GitHub branch protection]
        • check-neutral = unit-tests
        • check-skipped = unit-tests
        • check-success = unit-tests
      • #approved-reviews-by >= 2 [🛡 GitHub branch protection]
      • #approved-reviews-by>=2
      • #changes-requested-reviews-by = 0 [🛡 GitHub branch protection]
      • #changes-requested-reviews-by=0
      • #review-threads-unresolved = 0 [🛡 GitHub branch protection]
      • #review-threads-unresolved=0
      • -conflict
      • -draft
      • base=master
      • label!=🤖-do-not-merge
      • label=🤖-automerge
      • any of: [🛡 GitHub branch protection]
        • check-skipped = deploy to dockerhub
        • check-neutral = deploy to dockerhub
        • check-success = deploy to dockerhub
      • any of: [🛡 GitHub branch protection]
        • check-success = system-tests
        • check-neutral = system-tests
        • check-skipped = system-tests
      • any of: [🛡 GitHub branch protection]
        • check-success = check OAS' are up to date
        • check-neutral = check OAS' are up to date
        • check-skipped = check OAS' are up to date
      • any of: [🛡 GitHub branch protection]
        • check-success = integration-tests
        • check-neutral = integration-tests
        • check-skipped = integration-tests
      • any of: [🛡 GitHub branch protection]
        • check-success = build-test-images (frontend) / build-test-images
        • check-neutral = build-test-images (frontend) / build-test-images
        • check-skipped = build-test-images (frontend) / build-test-images
      • any of: [🛡 GitHub branch protection]
        • check-success = SonarCloud Code Analysis
        • check-neutral = SonarCloud Code Analysis
        • check-skipped = SonarCloud Code Analysis
  • -closed [📌 queue requirement]
  • -conflict [📌 queue requirement]
  • -draft [📌 queue requirement]
  • any of: [📌 queue -> configuration change requirements]
    • -mergify-configuration-changed
    • check-success = Configuration changed

Copy link
Contributor

@GitHK GitHK left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess ooil does not complain when you have added this new field.

@sanderegg sanderegg force-pushed the i1975/unlock-collaborative-nodes branch from aa154a9 to a18226e Compare August 22, 2025 08:24
@sanderegg
Copy link
Member Author

@mergify queue

@mergify
Copy link
Contributor

mergify bot commented Aug 22, 2025

queue

🟠 Waiting for conditions to match

  • -closed [📌 queue requirement]
  • -conflict [📌 queue requirement]
  • -draft [📌 queue requirement]
  • any of: [📌 queue -> configuration change requirements]
    • -mergify-configuration-changed
    • check-success = Configuration changed
  • any of: [🔀 queue conditions]
    • all of: [📌 queue conditions of queue default]
      • #approved-reviews-by >= 2 [🛡 GitHub branch protection]
      • #approved-reviews-by>=2
      • #changes-requested-reviews-by = 0 [🛡 GitHub branch protection]
      • #changes-requested-reviews-by=0
      • #review-threads-unresolved = 0 [🛡 GitHub branch protection]
      • #review-threads-unresolved=0
      • -conflict
      • -draft
      • base=master
      • branch-protection-review-decision = APPROVED [🛡 GitHub branch protection]
      • label!=🤖-do-not-merge
      • label=🤖-automerge
      • any of: [🛡 GitHub branch protection]
        • check-skipped = deploy to dockerhub
        • check-neutral = deploy to dockerhub
        • check-success = deploy to dockerhub
      • any of: [🛡 GitHub branch protection]
        • check-success = system-tests
        • check-neutral = system-tests
        • check-skipped = system-tests
      • any of: [🛡 GitHub branch protection]
        • check-success = unit-tests
        • check-neutral = unit-tests
        • check-skipped = unit-tests
      • any of: [🛡 GitHub branch protection]
        • check-success = check OAS' are up to date
        • check-neutral = check OAS' are up to date
        • check-skipped = check OAS' are up to date
      • any of: [🛡 GitHub branch protection]
        • check-success = integration-tests
        • check-neutral = integration-tests
        • check-skipped = integration-tests
      • any of: [🛡 GitHub branch protection]
        • check-success = build-test-images (frontend) / build-test-images
        • check-neutral = build-test-images (frontend) / build-test-images
        • check-skipped = build-test-images (frontend) / build-test-images
      • any of: [🛡 GitHub branch protection]
        • check-success = SonarCloud Code Analysis
        • check-neutral = SonarCloud Code Analysis
        • check-skipped = SonarCloud Code Analysis

@sonarqubecloud
Copy link

Copy link
Member

@pcrespov pcrespov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thx!

@sanderegg sanderegg merged commit c1c5d79 into ITISFoundation:master Aug 22, 2025
142 of 148 checks passed
@sanderegg sanderegg deleted the i1975/unlock-collaborative-nodes branch August 22, 2025 09:22
@matusdrobuliak66 matusdrobuliak66 mentioned this pull request Sep 2, 2025
61 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🤖-automerge marks PR as ready to be merged for Mergify a:webserver webserver's codebase. Assigning the area is particularly useful for bugs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Return node as unlocked if the service allows for collaborative mode

5 participants